Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #7231 Update base.css #7301

Closed
wants to merge 4 commits into from
Closed

Issue #7231 Update base.css #7301

wants to merge 4 commits into from

Conversation

v-lozko
Copy link
Contributor

@v-lozko v-lozko commented Mar 22, 2024

Updating to limit max width of inner windowed panel. Inner panel was extending past screen on right.

Added a max-width to the innerWindowedPanel as it was extending past 1200 px when window was larger than that.

Screenshot 2024-04-01 at 8 22 08 AM

Here is screenshot of notebook in binder

Screenshot 2024-04-01 at 8 36 22 AM

Updating to limit max width of inner windowed panel
Copy link
Contributor

Binder 👈 Launch a Binder on branch v-lozko/notebook/patch-1

@v-lozko v-lozko marked this pull request as draft March 22, 2024 01:06
@v-lozko v-lozko marked this pull request as ready for review March 22, 2024 01:07
@v-lozko
Copy link
Contributor Author

v-lozko commented Mar 22, 2024

Sorry, I forgot to add a PR label. As the description states this is related to issue 7231

@jtpio
Copy link
Member

jtpio commented Mar 29, 2024

Thanks @v-lozko for working on this.

Would you be able to add a screenshot to the top comment so it's easier to have an idea of the fix?

@jtpio jtpio added this to the 7.2.0 milestone Mar 29, 2024
@v-lozko
Copy link
Contributor Author

v-lozko commented Apr 1, 2024

Updated the top comment

@jtpio
Copy link
Member

jtpio commented Apr 2, 2024

Ah the failing UI tests may be relevant, as this change is affecting the default width for notebooks not using the full windowing mode:

image

@v-lozko
Copy link
Contributor Author

v-lozko commented Apr 2, 2024

I realized didn't actually take a screenshot of the actual notebook above. In any case, it seems to work appropriately when resizing it in both full and defer mode. It looks like its just a bit too wide. seems like only 693 pixels differ and some of those are the icons, and the height is 68.6px, so must be only a few pixels off on width. I set it to that variable because it seemed most appropriate.

What should be the next steps forward then to resolve this?

@jtpio
Copy link
Member

jtpio commented Apr 3, 2024

Thanks @v-lozko. I think we can trigger a snapshot update to fix the UI tests, and the PR should be ready to go.

@jtpio
Copy link
Member

jtpio commented Apr 3, 2024

bot please update playwright snapshots

@jtpio
Copy link
Member

jtpio commented Apr 3, 2024

Ah now that JupyterLab 4.2.0b0 is released and defaults to the full mode by default (jupyterlab/jupyterlab#15964), it looks like there is a misalignment of the notebook now.

@jtpio
Copy link
Member

jtpio commented Apr 5, 2024

FYI @v-lozko I picked up your change in #7312 directly (adding you as co-author of the commit) to facilitate the update to the newer JupyterLab release.

@v-lozko
Copy link
Contributor Author

v-lozko commented Apr 5, 2024

Awesome, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants